Skip to content

feat(recording): go2_mid360 + mid360_realsense_30 recorders#2588

Open
jeff-hykin wants to merge 7 commits into
mainfrom
jeff/feat/mid360_recorders
Open

feat(recording): go2_mid360 + mid360_realsense_30 recorders#2588
jeff-hykin wants to merge 7 commits into
mainfrom
jeff/feat/mid360_recorders

Conversation

@jeff-hykin

@jeff-hykin jeff-hykin commented Jun 24, 2026

Copy link
Copy Markdown
Member

go2_mid360 and mid360_realsense recording blueprints

Add memory2-based record blueprints for the Go2+Mid-360 and RealSense
D435i+Mid-360 rigs:

- PointlioPoseRecorder: shared Recorder base stamping each lidar frame
  with the latest odometry pose.
- StaticTfPublisher: republishes a rig's static mount frames onto /tf on
  an interval (PubSubTF has no latched static tf), so they're captured in
  the recording's tf stream.
- Go2Mid360Recorder / Mid360RealsenseRecorder + their static-transform
  trees and record blueprints (unitree-go2-mid360-record,
  mid360-realsense-record). Pygame WASD teleop on the go2 rig.
- Raw Livox capture is opt-in via RECORD_PCAP=1 (reuses the existing
  Mid360PcapRecorder); default off.
- Recording doc moved to experimental/docs/nav/map_recording/go2_mid360.md
  (post-processing stripped).

Regenerated all_blueprints.py.
@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds two new recording blueprints — go2_mid360 (Go2 robot + Livox Mid-360 + keyboard teleop) and mid360_realsense_30 (Mid-360 + RealSense D435i) — along with a shared PointlioPoseRecorder base class and a StaticTfPublisher helper that continuously re-publishes rigid mount transforms onto the tf stream during recording.

  • PointlioPoseRecorder: reusable base that stamps each lidar frame with the nearest odometry pose, now with a _POSE_MATCH_TOL = 0.1 s staleness guard so stale/dropped odometry leaves frames unposed (map-skipped) rather than mis-registered.
  • StaticTfPublisher: background loop that re-publishes a fixed set of Transform objects at a configurable rate; subclassed by rig-specific Go2Mid360StaticTf and Mid360RealsenseStaticTf with measured extrinsics.
  • Documentation (go2_mid360.md): the new setup guide instructs users to export LIDAR_IP=… in step 4, but none of the modules read that variable — the correct names are DIMOS_MID360_LIDAR_IP and DIMOS_POINTLIO_LIDAR_IP as specified in the Python docstring.

Confidence Score: 4/5

The recording pipeline itself is solid, but the setup guide gives users wrong environment variable names that would leave the lidar disconnected without any obvious error.

The new Python modules are well-structured and the staleness guard in PointlioPoseRecorder is correctly implemented. The documentation for the Go2+Mid-360 workflow (go2_mid360.md) instructs users to set LIDAR_IP, a variable that no module in the pipeline reads; the actual required vars are DIMOS_MID360_LIDAR_IP and DIMOS_POINTLIO_LIDAR_IP. A user following the guide verbatim would silently run with the wrong (default) lidar address on both the main and pcap-variant commands.

experimental/docs/nav/map_recording/go2_mid360.md — both code blocks in step 4 use the wrong env var name.

Important Files Changed

Filename Overview
dimos/hardware/sensors/lidar/pointlio/pose_recorder.py New base recorder class that stamps lidar frames with live odometry pose; correctly implements the _POSE_MATCH_TOL staleness guard — no issues found.
dimos/protocol/tf/static_tf_publisher.py New module that repeatedly publishes static transforms; _publish_loop has no exception handling on tf.publish(), flagged in a previous review cycle.
experimental/docs/nav/map_recording/go2_mid360.md New user guide; step 4 instructs users to export LIDAR_IP which is not read by any module — the correct vars are DIMOS_MID360_LIDAR_IP and DIMOS_POINTLIO_LIDAR_IP, same mistake in pcap snippet.
dimos/hardware/sensors/lidar/mid360_realsense_30/blueprints.py New blueprint wiring RealSense + Mid-360 rig; autoconnect remappings and pcap variant look correct, no issues found.
dimos/robot/unitree/go2/blueprints/basic/unitree_go2_mid360_record.py Drive-and-record blueprint using correct DIMOS_MID360_LIDAR_IP / DIMOS_POINTLIO_LIDAR_IP env vars; timezone now resolved dynamically. No issues in code.
dimos/hardware/sensors/lidar/mid360_realsense_30/recorder.py Thin subclass of PointlioPoseRecorder adding RealSense In ports; clean, no issues.
dimos/hardware/sensors/lidar/mid360_realsense_30/static_transforms.py Static mount-frame tree for the RealSense/Mid-360 rig, sourced from D435 xacro and Mid-360 extrinsics; no logic issues.
dimos/robot/unitree/go2/go2_mid360_recorder.py Thin subclass adding Go2-specific In ports (go2_lidar, go2_odom, color_image); clean.
dimos/robot/unitree/go2/go2_mid360_static_transforms.py Static mount frames for Go2 + Mid-360 measured on the physical rig; no logic issues.
dimos/robot/all_blueprints.py Registers four new blueprints/modules in the lookup tables; all module paths match the new files.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Mid360
    participant PointLio
    participant PR as PointlioPoseRecorder
    participant STF as StaticTfPublisher
    participant DB as memory2 db
    participant TF as tf stream

    Mid360->>PointLio: livox_lidar / livox_imu
    PointLio->>PR: pointlio_odometry → _odom_pose (stores pose + ts)
    PointLio->>PR: pointlio_lidar → _lidar_pose (staleness check vs _POSE_MATCH_TOL)
    PR->>DB: lidar frame + pose (or unposed if stale)
    PR->>DB: companion streams (camera, go2_lidar, etc.)

    loop every 1/publish_hz seconds
        STF->>TF: publish static mount transforms (re-stamped)
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Mid360
    participant PointLio
    participant PR as PointlioPoseRecorder
    participant STF as StaticTfPublisher
    participant DB as memory2 db
    participant TF as tf stream

    Mid360->>PointLio: livox_lidar / livox_imu
    PointLio->>PR: pointlio_odometry → _odom_pose (stores pose + ts)
    PointLio->>PR: pointlio_lidar → _lidar_pose (staleness check vs _POSE_MATCH_TOL)
    PR->>DB: lidar frame + pose (or unposed if stale)
    PR->>DB: companion streams (camera, go2_lidar, etc.)

    loop every 1/publish_hz seconds
        STF->>TF: publish static mount transforms (re-stamped)
    end
Loading

Reviews (6): Last reviewed commit: "refactor(recording): compose blueprints ..." | Re-trigger Greptile

Comment on lines +43 to +47
config: PointlioPoseRecorderConfig

pointlio_odometry: In[Odometry]
pointlio_lidar: In[PointCloud2]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 No staleness guard on _lidar_pose — stale odometry silently mis-stamps frames

_lidar_pose returns _last_odom_pose unconditionally, with no check on how old that pose is. If Point-LIO temporarily drops its odometry output (degenerate geometry, topic lag, process hiccup), every subsequent lidar frame will be stamped with the last known pose rather than None. None causes the frame to be map-skipped, which is the correct fallback; a stale pose causes it to be registered at the wrong location, silently corrupting the map.

The existing PointlioRecorder uses _POSE_MATCH_TOL = 0.1 s on the raw sensor timestamps (abs(raw_ts - self._last_odom_raw_ts) <= _POSE_MATCH_TOL) to detect exactly this case. The same guard — tracking _last_odom_raw_ts and comparing it against the lidar frame's raw ts — should be applied here so the behavior is consistent.

Comment on lines +34 to +37
class Go2Mid360Recorder(PointlioPoseRecorder):
pointlio_odometry: In[Odometry]
pointlio_lidar: In[PointCloud2]
go2_lidar: In[PointCloud2]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 These two ports are already declared on PointlioPoseRecorder; re-declaring them here is redundant and could confuse readers into thinking the subclass owns them. The same applies to Mid360RealsenseRecorder.

Suggested change
class Go2Mid360Recorder(PointlioPoseRecorder):
pointlio_odometry: In[Odometry]
pointlio_lidar: In[PointCloud2]
go2_lidar: In[PointCloud2]
class Go2Mid360Recorder(PointlioPoseRecorder):
go2_lidar: In[PointCloud2]

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


def _default_recording_dir() -> Path:
now = datetime.now()
stamp = now.strftime("%Y-%m-%d") + "_" + now.strftime("%I-%M%p").lower() + "-PST"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The -PST suffix is hardcoded regardless of the machine's actual timezone. datetime.now() returns local time, so on a machine set to UTC or any other zone the label is misleading. The same issue exists in mid360_realsense_30/mid360_realsense_record.py at the same line. The simplest fix is to drop the suffix, or use datetime.now().astimezone().strftime("%Z") to pick up the real zone abbreviation.

Suggested change
stamp = now.strftime("%Y-%m-%d") + "_" + now.strftime("%I-%M%p").lower() + "-PST"
stamp = now.strftime("%Y-%m-%d") + "_" + now.strftime("%I-%M%p").lower()

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.37569% with 50 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/protocol/tf/static_tf_publisher.py 51.02% 24 Missing ⚠️
...s/hardware/sensors/lidar/pointlio/pose_recorder.py 62.50% 12 Missing ⚠️
.../go2/blueprints/basic/unitree_go2_mid360_record.py 63.63% 10 Missing and 2 partials ⚠️
...ors/lidar/mid360_realsense_30/static_transforms.py 96.15% 1 Missing ⚠️
.../robot/unitree/go2/go2_mid360_static_transforms.py 90.00% 1 Missing ⚠️
@@            Coverage Diff             @@
##             main    #2588      +/-   ##
==========================================
+ Coverage   69.61%   70.76%   +1.15%     
==========================================
  Files         878      875       -3     
  Lines       79326    77622    -1704     
  Branches     7126     6893     -233     
==========================================
- Hits        55220    54929     -291     
+ Misses      22301    20889    -1412     
+ Partials     1805     1804       -1     
Flag Coverage Δ
OS-ubuntu-24.04-arm 62.91% <72.37%> (-0.08%) ⬇️
OS-ubuntu-latest 65.73% <72.37%> (-0.10%) ⬇️
Py-3.10 65.73% <72.37%> (-0.09%) ⬇️
Py-3.11 65.73% <72.37%> (-0.09%) ⬇️
Py-3.12 65.73% <72.37%> (-0.10%) ⬇️
Py-3.13 65.73% <72.37%> (-0.10%) ⬇️
Py-3.14 65.74% <72.37%> (-0.10%) ⬇️
Py-3.14t 65.72% <72.37%> (-0.10%) ⬇️
SelfHosted-Large 30.10% <ø> (+<0.01%) ⬆️
SelfHosted-Linux 37.80% <ø> (+<0.01%) ⬆️
SelfHosted-macOS 36.59% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...re/sensors/lidar/mid360_realsense_30/blueprints.py 100.00% <100.00%> (ø)
...ware/sensors/lidar/mid360_realsense_30/recorder.py 100.00% <100.00%> (ø)
dimos/robot/all_blueprints.py 100.00% <ø> (ø)
dimos/robot/unitree/go2/go2_mid360_recorder.py 100.00% <100.00%> (ø)
...ors/lidar/mid360_realsense_30/static_transforms.py 96.15% <96.15%> (ø)
.../robot/unitree/go2/go2_mid360_static_transforms.py 90.00% <90.00%> (ø)
...s/hardware/sensors/lidar/pointlio/pose_recorder.py 62.50% <62.50%> (ø)
.../go2/blueprints/basic/unitree_go2_mid360_record.py 63.63% <63.63%> (ø)
dimos/protocol/tf/static_tf_publisher.py 51.02% <51.02%> (ø)

... and 16 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…z label

Address Greptile review on PR #2588:
- pose_recorder: guard pointlio_lidar pose-stamping with _POSE_MATCH_TOL (0.1s)
  on raw odom ts, matching PointlioRecorder. Stale odometry now yields an
  unposed (map-skipped) frame instead of mis-stamping at the last pose.
- go2/realsense recorders: drop redundant pointlio_odometry/pointlio_lidar
  port re-declarations (inherited from PointlioPoseRecorder).
- record blueprints: use datetime.now().astimezone() + %Z for the recordings
  dir label instead of a hardcoded -PST suffix.
mid360_realsense_30/__init__.py was license-header only; the repo forbids
__init__.py under dimos/. Namespace-package import still works.
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Jun 24, 2026
Rename mid360_realsense_record.py -> blueprints.py, drop the __main__ runner,
and expose two blueprints instead of the RECORD_PCAP env toggle:
- mid360_realsense_record (db only)
- mid360_realsense_record_with_pcap (db + raw Mid-360 pcap)

Matches the repo's blueprints.py convention (e.g. virtual_mid360). Regenerated
all_blueprints.py.
@github-actions github-actions Bot added ready-to-merge Required CI checks have passed on this PR and removed ready-to-merge Required CI checks have passed on this PR labels Jun 24, 2026
…s consts

Let each module self-configure its lidar IP from its own env var
(DIMOS_MID360_LIDAR_IP for Mid360/pcap, DIMOS_POINTLIO_LIDAR_IP for Point-LIO)
instead of a blueprint-level LIDAR_IP env + hardcoded default fanned out to all
of them. Inline n_workers. Applies to both the realsense blueprints and the go2
record blueprint.
@github-actions github-actions Bot added ready-to-merge Required CI checks have passed on this PR and removed ready-to-merge Required CI checks have passed on this PR labels Jun 24, 2026
Build the with-pcap variant by nesting the base blueprint
(autoconnect(base, Mid360PcapRecorder)) instead of spreading a shared *_modules
list. autoconnect merges the nested blueprint's modules, remappings, and
global_config (n_workers) — verified. Same nesting for the go2 RECORD_PCAP opt-in.
@github-actions github-actions Bot added ready-to-merge Required CI checks have passed on this PR and removed ready-to-merge Required CI checks have passed on this PR labels Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant